Skip to content

Adjust thread exec de-threading to reduce false fatal exits under load - #309

Merged
jserv merged 5 commits into
mainfrom
fixes
Aug 19, 2026
Merged

Adjust thread exec de-threading to reduce false fatal exits under load#309
jserv merged 5 commits into
mainfrom
fixes

Conversation

@jserv

@jserv jserv commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Summary by cubic

Removes the 1s “no-departure” stall from threaded exec de-threading and bounds the wait by a single 10s wall-clock ceiling to prevent false fatal exits under load; the loop re-pokes siblings each iteration. Completes and enforces the lock-order document and hardens the default build so CI fails on undocumented locks and compiler warnings.

  • Adds scripts/check-lock-order.py to make check with self-tests; fails if any file-scope pthread_mutex_t/pthread_rwlock_t under src/ is missing from the lock-order block, if the document names a non-existent lock, or if the same lock name is defined in more than one file. Updates docs/testing.md and mk/tests.mk.
  • Expands and corrects the lock-order block in src/syscall/internal.h: names all file-scope locks, adds an explicit leaf list, places log_mutex below the entire order, uses exec_handoff_lock, and fixes several positions.
  • Treats warnings as errors by default in mk/config.mk (WERROR=1), enables -fstack-protector-strong, and sets _FORTIFY_SOURCE=2 except under AddressSanitizer (sanitizers parsed by exact name). Fixes tests/test-casefold-walk-host.c to avoid -Wstring-plus-int.

Rollout/Migration

  • When adding a file-scope pthread_mutex_t/pthread_rwlock_t (including in headers), add it to the ordered or leaf list in src/syscall/internal.h; otherwise make check fails.
  • Do not define the same mutex/rwlock name in multiple source files; the check rejects collisions.
  • Fix new compiler warnings or build locally with WERROR=0; CI keeps WERROR=1.
  • De-threading may now use the full 10s budget; update tests that assumed the prior 1s stall.

Written for commit ee41153. Summary will update on new commits.

Review in cubic

@jserv jserv changed the title Fixes Adjust thread exec de-threading to reduce false fatal exits under load Aug 19, 2026
cubic-dev-ai[bot]

This comment was marked as resolved.

cubic-dev-ai[bot]

This comment was marked as resolved.

cubic-dev-ai[bot]

This comment was marked as resolved.

jserv added 4 commits August 19, 2026 17:54
thread_exec_de_thread gave up on its siblings after 1000 ms with no
departure. Departures are not a liveness signal: once one sibling is
left there are none to observe, so the interval measures that thread's
own latency, which is the fixed bound it replaced. Sizing it honestly
means covering the slowest re-check quantum a parked sibling owes
(200 ms, in the io wait) times how far behind schedule the host runs,
and that multiplier is not a property this code can know.

An ASAN make check on a machine running the sanitizer lanes of several
workflow runs at once reported one sibling still inside mmap after
1005 ms, last departure at 4 ms, and sys_execve took its post-PNR fatal
exit. The join below it, 100 iterations of usleep(5000) and nominally
500 ms, took 9 s in the same run.

The ceiling is the only bound, which keeps the budget in the loop that
can spend it: this one re-issues the whole wake set every iteration and
the join only polls.
The warning set in mk/config.mk is strong and every diagnostic only
printed. The gates around it, syscall coverage and the EINTR contract
and the proof targets, all fail the build instead, so a compiler
warning was the one signal a reader had to notice unaided. WERROR=0
turns it off, which is what a newer compiler with a new warning wants:
the flag must not be the reason a fresh clone stops building.

tests/test-casefold-walk-host.c indexed a string literal by a size_t
walk offset twice, which -Wstring-plus-int reads as an attempt to
concatenate. The arithmetic is right and the spelling is not, so the
literal takes a name and the offsets index an array.
This process parses input the guest fully controls: its ELF, every
syscall argument, FUSE frames, netlink messages, sockaddr and cmsg
blobs. The bounds math for those is proved in src/proved/, and the
proofs cover the arithmetic rather than the call sites consuming it.
PIE is already the Darwin default; -fstack-protector-strong and
_FORTIFY_SOURCE are not.

_FORTIFY_SOURCE is skipped under AddressSanitizer alone, which
predefines it to 0 on purpose since its interceptors do the same job,
so redefining it is a -Wmacro-redefined error under the -Werror above.
UBSAN and TSAN predefine nothing and keep it, which is what makes those
lanes exercise the same libc entry points the shipped binary calls.

Each -fsanitize= argument is split on commas and matched by name. A
substring test for -fsanitize=address is order-dependent while reading
as if it were not: it answers correctly for -fsanitize=address,undefined
and wrongly for -fsanitize=undefined,address, the same request spelled
the other way round.
The block at the top of src/syscall/internal.h documented an 11-entry
acquisition order and presented it as the ordering. Two of those entries
name a per-instance lock by role, so it covered 9 of the tree's 31
file-scope locks, and seven of the twenty-two it left out are held
across another acquisition: autoreap_lock over pid_lock and pidfd_lock,
elf_path_lock over cwd_lock and sysroot_lock, oom_write_lock over
fd_lock, fuse_lock over the per-session lock, proc_tmpdir_lock over
mmap_lock, pty_keepalive_lock over fd_lock, and cwd_lock over fuse_lock.
Each is correct as written, and a reader adding the next lock had no
rule to consult.

The FUSE per-session lock reaches fd_lock and sig_lock through
asyncio_fire, so the pairing with fuse_lock is not the whole of its
ordering. log_mutex sits below the entire order rather than beside the
other leaves, since any lock may log while held. sig_lock is not a leaf:
signal_queue_thread_common takes thread_lock under it.

A leaf list closes the set, so the two lists together are exhaustive: a
new lock belongs in one of them, and a leaf that grows a call to another
locker moves up. The numbered "Lock order: N" comments at the
definitions are not indices into this list; a number there records only
which locks that one was known to precede when it was written.
The lock-order block in src/syscall/internal.h claims to name every
lock, and prose cannot hold that claim on its own. An inversion is not
the kind of defect the suite finds: it needs the two threads to
interleave the one way that deadlocks, on a machine loaded enough to
make that likely, which describes CI and not a developer's laptop.

scripts/check-lock-order.py is a set comparison in both directions:
every file-scope pthread_mutex_t and pthread_rwlock_t under src/ is
named by the block, and every lock the block names still exists.
Headers are scanned too, because a static definition in one compiles
into every translation unit that includes it.

The declaration is captured whole and taken apart afterwards, because
no single pattern reads initializers correctly: a clause that stops at
the first comma drops the declarators behind it, one that runs to the
semicolon swallows them, and one that refuses braces skips a brace
initializer outright. The list splits at commas outside brackets and
each declarator drops its own initializer, so an array bound, a brace
initializer, a parameter list, and a declarator standing behind an
initializer all parse. Every one of those failures is a silent miss,
which is the one shape worse than no gate.

Which of the two lists a lock belongs in stays a human judgement. That
question needs the call graph, and a branch-insensitive answer is wrong
in both directions: a linear scan of fuse.c drops fuse_lock at an
early-return unlock and files a documented non-leaf as a leaf, while the
same scan reads two comments mentioning sys_close as an
sfd_lock/fd_lock cycle. A gate that cries wolf earns an exemption list
and then gets ignored.
@jserv
jserv merged commit bffd6bd into main Aug 19, 2026
18 checks passed
@jserv
jserv deleted the fixes branch August 19, 2026 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant